feat(sep-1932): DPoP refresh token binding - #524

A fully conformant refresh gets graded FAILURE here. To be precise about which flow, since there are two that look alike:

• Flow A — bound refresh token presented without a proof: correctly rejected below (invalid_grant, "DPoP proof required for a bound refresh token") and pinned by your unit test. No issue; this is §5's binding validation working.
• Flow B — this branch: the client did the code exchange with no DPoP proof at all, so it holds a Bearer token and an unbound refresh token. entry.jkt is undefined because there was never a key to bind. RFC 9449 §5's MUST only attaches when "a public client ... presents a valid DPoP proof at the token endpoint" at issuance — Flow B is plain RFC 6749 end to end, and a proof-less refresh of it violates nothing.

But this branch records {proofValid: false, jktMatched: false}, so refreshProofCheck() emits FAILURE with "Refresh request did not prove possession of the DPoP key bound at the authorization-code exchange" — no key was ever bound, and the check's own description scopes itself to "a bound refresh token". The client's one real defect (no token-request proof) is already correctly failing sep-1932-client-token-request-proof; this double-reports the same defect under the wrong check with a factually wrong message. One defect, one failure, right label.

Reproduced end-to-end: Bearer code exchange → proof-less refresh → 200 → check verdict FAILURE. The shipped suite never hits it because no example client combines a proof-less exchange with exerciseRefresh.

Minimal fix — an unbound refresh just isn't evidence about the binding check, so don't record it:

Verified with this change applied: tsc --noEmit clean, full suite 655/655, and the repro now lands SKIPPED (non-scoring). The one trade-off: an unbound refresh then reports identically to "client never refreshed". If you'd rather keep the trace, the richer alternative is a bound: false field on the observation that the check maps to SKIPPED with an accurate reason — happy with either.

Generated with the aid of Claude (Fable 5).

A refresh-binding violation during an auth/dpop-nonce run is invisible — no FAILURE, no SKIPPED, no check at all. This gate emits the refresh check only in the baseline posture, but both postures issue refresh tokens (issueRefreshTokens is set for both scenarios) and handleRefreshGrant fully supports the nonce flow. Reproduced: in the nonce posture, a wrong-key refresh — a real §5 violation the AS catches with invalid_grant — leaves no trace in the emitted checks. A client whose refresh defect only manifests under nonce handling (e.g. falling back to a proof-less refresh after a use_dpop_nonce challenge) escapes grading entirely. Your own doc comment at line 115 already promises the right behavior ("Emits the four baseline checks plus two more") — it's this line that doesn't deliver it:

One caveat: this is only safe together with the reordering suggested in createAuthServer.ts (handleRefreshGrant) — as the recording stands, a challenged-but-never-retried refresh writes an all-green observation before the nonce gate, which this change would promote to a false SUCCESS.

Generated with the aid of Claude (Fable 5).

Two ordering problems in this arm, fixed by one swap. First, the all-green observation is recorded before the nonce gate: a refresh that gets challenged and never retried leaves {proofValid: true, jktMatched: true} although no token was issued — an uncompleted refresh that reads as a completed one. Harmless today only because the nonce posture doesn't emit the refresh check (see the companion comment on dpop.ts line 223); the moment that's fixed, this becomes a false SUCCESS. Second, the key-binding mismatch is judged before the nonce challenge — the same sequencing we're reworking on #523's authorization_code arm: the AS should challenge a valid proof for its nonce first and judge key agreement only on the nonce-carrying retry, so one client defect doesn't produce verdicts off a pre-challenge request.

The fix is just the two blocks swapped (the mismatch block itself is byte-identical, only moved below the gate):

Generated with the aid of Claude (Fable 5).

This new row's sibling at lines 40-41 (outside the diff, so commenting here) is now false clause by clause. The excluded AS-side refresh-binding row still says: "Not exercisable today: the shared conformance test authorization server does not issue refresh tokens or handle the refresh_token grant, so a conformant-vs-misbehaving pair cannot be built to validate the check. Deferred until the test AS gains refresh support." This PR is precisely the test AS gaining refresh support — it issues and rotates refresh tokens, handles the refresh_token grant, and even ships the conformant-vs-misbehaving pair the rationale says cannot be built (the unbound-refresh and rebind-on-refresh misbehavior modes, currently exercised by unit tests). The yaml is the SEP's coverage-traceability record: leaving the old rationale in place misreports to anyone auditing coverage that AS-side refresh binding is untestable, when what's actually true after this PR is narrower — the fixture exists, but no authorization-server-under-test scenario uses it yet.

Suggested replacement for the excluded: text at line 41:

    excluded: 'AS-side half of refresh binding: the shared test AS now issues, rotates, and validates bound refresh tokens (and has unbound-refresh / rebind-on-refresh misbehavior modes), but an authorization-server-under-test scenario exercising the requirement has not yet been built. The client-side half is covered by sep-1932-client-refresh-proof.'

Generated with the aid of Claude (Fable 5).

A real client will never reach a state where it has to use its refresh token, so the new check cannot measure real clients yet — it grades only this PR's own fixtures. Tokens live 3600 seconds and a run lasts seconds: a proactive refresher's timer never fires, and a reactive refresher never sees a 401 invalid_token (the token genuinely isn't expired). Either way no refresh happens, the observation sink stays empty, and sep-1932-client-refresh-proof lands SKIPPED (non-scoring) for every client except the in-repo examples, which refresh unconditionally. The mechanism that would change this is described in this very comment — "The refresh posture sets this to 30 so a client must refresh during the run" — but no such posture exists (DpopClientPosture is 'baseline' | 'nonce') and nothing anywhere sets accessTokenExpiresIn.

To be clear, merging the refresh infrastructure first and the forcing posture second is a reasonable sequencing — the AS machinery and its unit tests are solid. The gap is now tracked as #551 (a short-expiry auth/dpop-refresh posture). The ask for this PR is just that it not ship the half-built trigger:

1. Trim this comment's last sentence — the posture it references doesn't exist yet; it's auth/dpop: refresh-binding check is unreachable for real clients — add a short-expiry posture #551.
2. Move the expired-token 401 gate in dpopResourceAuth.ts (lines 92-101 plus the accessTokenExpired helper) to auth/dpop: refresh-binding check is unreachable for real clients — add a short-expiry posture #551, where it becomes reachable and gets tests. It's unreachable in every current scenario and exercised by no test, and it's only half the trigger anyway — forcing real clients to refresh needs short expires_in (proactive refreshers) AND 401-on-expired (reactive ones), plus example clients that handle the invalid_token re-auth flow. It also carries a latent hazard to resolve in auth/dpop: refresh-binding check is unreachable for real clients — add a short-expiry posture #551 rather than inherit silently: the 401 returns before DPoP proof validation, so in a short-expiry run the proofs on expired-token requests would never be judged.

To be explicit about scope: none of this touches the refresh-binding enforcement or its tests — the AS-side proof validation on refresh, the check, the broken-client scenarios and all ten fixture unit tests are untouched by both asks.

Generated with the aid of Claude (Fable 5).

Every verdict this check emits carries a false statement in its details. The reason field — "Client did not use the optional refresh token" — is attached unconditionally, so a SUCCESS report says the client refreshed correctly and that it never refreshed, side by side (refreshSeen: true directly above it), and a FAILURE report blames a refresh the same details claim never happened. The sentence is only true on the SKIPPED branch, which is the convention elsewhere in the suite: a reason travels with the condition it explains (untestable checks pair it with untestable: true; the sibling checks in this file put only observation fields in details).

Generated with the aid of Claude (Fable 5).

These unit tests run every authorization-code exchange through a deliberately broken PKCE pair. 'x'/'x' is not a valid S256 pair ('x' is also well under RFC 7636 §4.1's 43-char verifier minimum), and this AS validates the pair — so each exchange in this file records a pkce-verifier-matches-challenge FAILURE into the locally-created checks array. The tests pass only because that array is never asserted on, which makes the fixture silently dependent on the AS observing-not-enforcing PKCE, and means the refresh behavior under test runs inside flows that are simultaneously failing an unrelated conformance check.

The fix is a real pair, used in three places. Add near the imports:

import { createHash } from 'node:crypto';

// A real S256 pair (RFC 7636 §4.1: verifier 43-128 chars), so these tests
// don't depend on the test AS tolerating an invalid PKCE exchange.
const CODE_VERIFIER = 'refresh-tests-code-verifier-0123456789-0123456789ab';
const CODE_CHALLENGE = createHash('sha256')
  .update(CODE_VERIFIER)
  .digest('base64url');

Then replace the three stub values: on this line, code_challenge: CODE_CHALLENGE; at lines 116 and 176, code_verifier: CODE_VERIFIER.

(The same change was requested on #523's independently-created copy of this file — the two versions will need unifying at merge time anyway, so whichever lands second can carry one shared constant.)

Generated with the aid of Claude (Fable 5).

This edited comment now misdescribes the test twice. The run emits four baseline checks, but this client never uses its refresh token, so the fourth (sep-1932-client-refresh-proof) is SKIPPED, not SUCCESS — and with this PR's testClient.ts change, SKIPPED checks are excluded from the default all-SUCCESS assertion entirely. So the test neither sees four SUCCESS checks nor asserts anything about the refresh check. That matters beyond wording: a reader trusting this comment could delete the dedicated refresh-success test above (the one with expectedSuccessSlugs) as redundant, silently losing the only SUCCESS assertion the refresh check has.

Generated with the aid of Claude (Fable 5).

The fields in this record combine across ALL refreshes, but the comment reads as if they describe one refresh. recordRefresh ANDs every field across every refresh the client performs and keeps the first error — so after one bad refresh, proofValid stays false no matter how many good refreshes follow. That's deliberate (it's what stops a later retry from hiding a violation, and the "keeps a failed refresh observation after a later valid retry" test pins it), but nothing in this comment or the field names says so: a reader will assume proofValid: false describes the client's most recent refresh. The other observation record above (DpopTokenRequestObservation) documents its equivalent behavior explicitly ("sticky-failure: once any exchange lacks a valid proof it stays FAILURE") — this one should too:

If the unbound-refresh change suggested further down lands (stop recording unbound refreshes), "bound and unbound" here should become "bound refreshes only" in the same pass.

Generated with the aid of Claude (Fable 5).